CLAUDE.md violation: The exception's message is manually interpolated into the log string.

Per CLAUDE.md: "NEVER manually append the Throwable's message or any other props to the string passed as the 1st param of Logger.* calls, its internals are already enriching the final log message with the details of the Throwable passed via the e arg."

Pass e as the second positional argument instead:

CLAUDE.md violation: _lines.value = ... is used directly in log() (and clear() below) instead of _lines.update { ... }.

Per CLAUDE.md: "ALWAYS use _uiState.update { }, NEVER use _stateFlow.value ="

Using .update {} is also safer for concurrency — it avoids the read-then-write race between .value and the assignment (even though this block is synchronized). Replace all three assignments with _lines.update { ... }.

CLAUDE.md violation: TrezorUiState (and SendStep below it) are declared before the TrezorViewModel class in this file.

Per CLAUDE.md: "ALWAYS create data classes for state AFTER viewModel class in same file."

Move TrezorUiState and SendStep to below the TrezorViewModel class definition.

CLAUDE.md violation: Inline fully-qualified class names are used here (and on the Change and OpReturn branches below) instead of imports.

Per CLAUDE.md: "ALWAYS add imports instead of inline fully-qualified names."

Add import com.synonym.bitkitcore.TrezorPrecomposedOutput at the top of the file and use the short names (TrezorPrecomposedOutput.Payment, TrezorPrecomposedOutput.Change, TrezorPrecomposedOutput.OpReturn). Other files in this PR (e.g. BalanceLookupSection.kt) already import and use the short name.

Bug: Thread.sleep on the BLE GATT callback thread.

Thread.sleep(200) is called inside onDescriptorWrite, which Android invokes on its single internal BLE GATT callback thread. Blocking this thread for 200 ms delays all subsequent BLE GATT callbacks (e.g. onCharacteristicChanged, onCharacteristicWrite) for all active connections until the sleep completes. This can cause characteristic notification timeouts (your read timeout is 5000 ms, but any notification arriving in this window will be queued and potentially misattributed) and intermittent connection failures.

If a short stabilization delay is truly necessary after enabling CCCD, perform it on a separate worker thread rather than blocking the GATT callback dispatcher.

CLAUDE.md violation: settings__adv__section_hardware_wallet is inserted after settings__adv__section_payments, but alphabetically h < n < o < p, so it should appear before settings__adv__section_networks.

Per CLAUDE.md: "ALWAYS add new localizable string string resources in alphabetical order in strings.xml."

CLAUDE.md violation / latent crash risk: uiState.precomposedResult!! (and uiState.signedTxResult!! a few lines below) use the non-null assertion operator on nullable fields.

Per CLAUDE.md: val result = nullable!!.doSomething() is listed under ❌ DON'T — use safe calls instead.

While the ViewModel currently transitions sendStep and the result field atomically in a single update {} call, the type system does not enforce this contract. Any future code path that transitions sendStep without also setting the corresponding result will crash at runtime with no warning. Prefer safe calls with an early return:

unstable parameter

Bug: userInitiatedClose is a single shared flag across all BLE connections

This @Volatile boolean is not per-connection — it is shared across all entries in bleConnections. With two concurrent BLE connections, closing device A sets the flag to true. If device B disconnects externally before device A's GATT callback fires, B's onConnectionStateChange reads userInitiatedClose == true and suppresses the _externalDisconnect emission for B. The flag is then reset to false, causing A's subsequent callback to spuriously emit _externalDisconnect.

Since closeAllConnections() iterates over all entries and calls closeBleDevice for each, multi-device scenarios are expected and this race is reachable.

Suggested fix: replace the shared boolean with a per-path set, e.g.:

private val userInitiatedCloseSet = ConcurrentHashMap.newKeySet<String>()

// In closeBleDevice:
userInitiatedCloseSet.add(path)

// In onConnectionStateChange:
if (!userInitiatedCloseSet.remove(path)) {
    _externalDisconnect.tryEmit(path)
}

Related code:

Bug: Thread.sleep(200) blocks the Android BLE callback thread

All BluetoothGattCallback methods are dispatched on a single internal Android Bluetooth thread. Calling Thread.sleep(200) here blocks that thread for 200 ms, stalling all queued GATT events for every active BLE connection — including onCharacteristicWrite and onCharacteristicChanged needed by read/write operations in this transport. Under normal operating conditions (sequential descriptor writes followed by characteristic reads), this directly contributes to the 5000 ms read/write timeouts firing.

Move any delay-based stabilization to a coroutine dispatcher and signal the waiting code via a CountDownLatch or Channel from the callback, rather than sleeping on the callback thread.

Related code:

Bug: autoReconnect connects to any USB Trezor regardless of known-device identity

val usbDevice = scannedDevices.find { it.transportType == TrezorTransportType.USB }
val idMatch = knownDevices.firstNotNullOfOrNull { known -> scannedDevices.find { it.id == known.id } }
val match = usbDevice ?: idMatch ?: error("No known device found nearby")

The usbDevice fallback picks the first USB device found unconditionally, without verifying that its ID matches any known/paired device. idMatch (which verifies by ID) is only used as a fallback when no USB device is present at all.

In a hardware wallet context this is a correctness and security concern: a different device carries different keys and accounts.

Suggested fix — invert the priority:

Related code:

Bug: SharedPreferences.commit() blocks the calling thread

commit() is synchronous — it writes to disk and blocks the thread until done. saveKnownDevices() is not a suspend function and does not dispatch to an IO dispatcher, so this blocking write runs on the coroutine dispatcher thread that called connect(), autoReconnect(), or forgetDevice().

Since the boolean return value of commit() is unused here, replace with apply():

Related code:

CLAUDE.md: Screen-specific ViewModel placed in viewmodels/

TrezorViewModel is only consumed by TrezorScreen and its sub-composables under ui/screens/trezor/. Per the project rule:

ALWAYS co-locate screen-specific ViewModels in the same package as their screen; only place ViewModels in viewmodels/ when shared across multiple screens

This file should be at app/src/main/java/to/bitkit/ui/screens/trezor/TrezorViewModel.kt.

See: https://github.com/synonymdev/bitkit-android/blob/463750843ad62fe87d961df909faf0c174f6f52d/CLAUDE.md

CLAUDE.md: State data class declared before Repo class

TrezorState (line 42) is declared before class TrezorRepo (line 58). Per the project rule:

ALWAYS create data classes for state AFTER viewModel class in same file

Move TrezorState to after the closing brace of TrezorRepo.

See: https://github.com/synonymdev/bitkit-android/blob/463750843ad62fe87d961df909faf0c174f6f52d/CLAUDE.md

CLAUDE.md: Raw getSystemService() casts instead of ext/Context.kt extensions

Per the project rule:

ALWAYS use or create Context extension properties in ext/Context.kt instead of raw context.getSystemService() casts

Add to ext/Context.kt:

val Context.usbManager: UsbManager get() = getSystemService(Context.USB_SERVICE) as UsbManager
val Context.bluetoothManager: BluetoothManager get() = getSystemService(Context.BLUETOOTH_SERVICE) as BluetoothManager

See: https://github.com/synonymdev/bitkit-android/blob/463750843ad62fe87d961df909faf0c174f6f52d/CLAUDE.md

CLAUDE.md: e.message manually appended to Logger call string

Per the project rule:

NEVER manually append the Throwable's message or any other props to the string passed as the 1st param of Logger.* calls, its internals are already enriching the final log message with the details of the Throwable passed via the e arg

See: https://github.com/synonymdev/bitkit-android/blob/463750843ad62fe87d961df909faf0c174f6f52d/CLAUDE.md

CLAUDE.md: Hardcoded strings in ViewModel

All ToastEventBus.send(title = "...") calls throughout this ViewModel use hardcoded English strings ("Reconnected to $label", "Trezor initialized", "Connected to $label", "Forgot $name", "Address generated", "Transaction broadcast", etc.).

Per the project rules:

NEVER hardcode strings and always preserve string resources
ALWAYS localize in ViewModels using injected @ApplicationContext, e.g. context.getString()

Note: the rule NEVER add string resources for strings used only in dev settings screens only exempts adding entries to strings.xml — it does not exempt the ViewModel from using context.getString(). The ViewModel currently has no @ApplicationContext injection.

See: https://github.com/synonymdev/bitkit-android/blob/463750843ad62fe87d961df909faf0c174f6f52d/CLAUDE.md

CLAUDE.md: Inline fully-qualified class name instead of import

Per the project rule:

ALWAYS add imports instead of inline fully-qualified names

Add import java.io.File to the imports section and use File(...) directly.

See: https://github.com/synonymdev/bitkit-android/blob/463750843ad62fe87d961df909faf0c174f6f52d/CLAUDE.md

Bug: Thread.sleep() called on GATT callback thread

Thread.sleep(200) is called directly inside onDescriptorWrite, which runs on the Android Bluetooth GATT callback thread. The GATT callback thread is serialized — blocking it prevents all subsequent GATT events (onCharacteristicChanged, onCharacteristicWrite, onConnectionStateChange, etc.) from being delivered until the sleep ends.

This will cause:

• Write latches (writeLatch.await()) to see artificial delays or time out
• Read queues to stall while new onCharacteristicChanged notifications are blocked
• Connection state changes to be missed during the 200ms window

The delay should be moved off the callback thread, e.g., by posting work to a background thread or coroutine.

Bug: userInitiatedClose is shared across all BLE device connections

userInitiatedClose is a single class-level @Volatile flag but bleConnections is a ConcurrentHashMap that holds multiple simultaneous BLE connections. When two devices disconnect concurrently (e.g. via closeAllConnections()):

1. Thread A sets userInitiatedClose = true for device A, calls gatt.disconnect()
2. Thread B sets userInitiatedClose = true for device B, calls gatt.disconnect()
3. GATT callback fires for device A: reads true ✓, resets to false
4. GATT callback fires for device B: reads false → incorrectly emits to _externalDisconnect

Similarly, an actual external disconnect from device B can be silently swallowed if device A is being closed at the same time.

Fix: move userInitiatedClose into the BleConnection data class so it is scoped per-device.

Bug: autoReconnect connects to any USB Trezor, not just known/trusted devices

usbDevice is selected from scanned devices by transport type alone — it is not cross-referenced against knownDevices. Since usbDevice ?: idMatch gives USB unconditional priority, any Trezor plugged in via USB (including one never previously paired) will be auto-connected as if it were the user's trusted device.

This undermines the trust model of the hardware wallet: an attacker with physical USB access could substitute their own Trezor and have it silently treated as the user's wallet.

Fix: filter usbDevice to only match if the device is also in knownDevices:

val usbDevice = scannedDevices.find { device ->
    device.transportType == TrezorTransportType.USB &&
    knownDevices.any { it.id == device.id }
}

CLAUDE.md: Use imports instead of inline fully-qualified names

Per CLAUDE.md: "ALWAYS add imports instead of inline fully-qualified names"

com.synonym.bitkitcore.TrezorTransportType is referenced inline here and in at least one other location in this file. Add an import at the top of the file and use the short name TrezorTransportType instead.

CLAUDE.md: Use imports instead of inline fully-qualified names

Per CLAUDE.md: "ALWAYS add imports instead of inline fully-qualified names"

com.synonym.bitkitcore.TrezorPrecomposedOutput is referenced inline for Payment, Change, and OpReturn subtypes. Add import statements and use TrezorPrecomposedOutput.Payment, etc.

CLAUDE.md: Use imports instead of inline fully-qualified names

Per CLAUDE.md: "ALWAYS add imports instead of inline fully-qualified names"

androidx.compose.ui.res.painterResource is used inline here (and in at least one other place in this file). Add import androidx.compose.ui.res.painterResource and use the short name.

CLAUDE.md: Screen-specific ViewModel should be co-located with its screen

Per CLAUDE.md: "ALWAYS co-locate screen-specific ViewModels in the same package as their screen; only place ViewModels in viewmodels/ when shared across multiple screens"

TrezorViewModel is only used by TrezorScreen (ui/screens/trezor/). Since it is not shared across multiple screens, it should be moved to app/src/main/java/to/bitkit/ui/screens/trezor/TrezorViewModel.kt.

CLAUDE.md: Use Context extension properties instead of raw getSystemService() casts

Per CLAUDE.md: "ALWAYS use or create Context extension properties in ext/Context.kt instead of raw context.getSystemService() casts"

Both usbManager and bluetoothManager are accessed via raw casts here. Neither extension property exists yet in ext/Context.kt. Add them:

// ext/Context.kt
val Context.usbManager: UsbManager
    get() = getSystemService(Context.USB_SERVICE) as UsbManager

val Context.bluetoothManager: BluetoothManager
    get() = getSystemService(Context.BLUETOOTH_SERVICE) as BluetoothManager

Then reference context.usbManager and context.bluetoothManager here.

CLAUDE.md: State data class should be declared after the ViewModel class

Per CLAUDE.md: "ALWAYS create data classes for state AFTER viewModel class in same file"

TrezorUiState and SendStep are declared before TrezorViewModel. Move them to after the TrezorViewModel class definition.

CLAUDE.md: No string resources for dev-only settings screens

Per CLAUDE.md: "NEVER add string resources for strings used only in dev settings screens and previews"

settings__adv__section_hardware_wallet is exclusively used inside an if (isDevModeEnabled) block in AdvancedSettingsScreen. Since this string is only ever visible under dev mode, it should be a hardcoded string literal (e.g. "Hardware Wallet") rather than a string resource. The same applies to settings__adv__trezor.

Bug: race condition in closeBleDevice — latch may never fire

The connection is removed from bleConnections at line 820, then re-inserted with a new CountDownLatch at line 826. This is a non-atomic compound operation. If onConnectionStateChange(STATE_DISCONNECTED) fires on the GATT callback thread in the window between the remove and the re-insert, bleConnections[path] returns null, connection?.disconnectLatch?.countDown() is a no-op, and the latch is never counted down — causing closeBleDevice to always block for the full DISCONNECT_TIMEOUT_MS (3 seconds) before timing out.

Fix: create the new latch and insert the updated connection before calling gatt.disconnect(), without removing first:

Bug: ACCESS_FINE_LOCATION missing maxSdkVersion="30"

BLUETOOTH and BLUETOOTH_ADMIN are correctly capped at maxSdkVersion="30", but ACCESS_FINE_LOCATION has no cap. On Android 12+ (SDK >= 31), BLUETOOTH_SCAN with neverForLocation is declared and location access is not required for BLE scanning — so this permission is over-declared on modern devices and will trigger an unnecessary location permission prompt.

Bug: SIGNET incorrectly mapped to "Testnet"

Trezor firmware and the bitkitcore library treat "Signet" and "Testnet" as distinct coin types with different network parameters (genesis block, signet challenge script, address format). Passing "Testnet" for a Signet wallet will cause address derivation mismatches or a firmware rejection at transaction-compose time. The coinStr from this function is passed directly as the coin field in TrezorPrecomposeParams.

CLAUDE.md: inline fully-qualified names instead of imports

Per CLAUDE.md: "ALWAYS add imports instead of inline fully-qualified names".

Two occurrences in this file:

1. Line 336: kotlinx.coroutines.flow.StateFlow<Boolean> — StateFlow is already imported elsewhere in the file; drop the qualifier here.
2. Line 1123: android.os.Handler(android.os.Looper.getMainLooper()) — both Handler and Looper should be imported and referenced by simple name.

CLAUDE.md: string resources added for dev-only settings screens

Per CLAUDE.md: "NEVER add string resources for strings used only in dev settings screens and previews and never localize acronyms".

Both settings__adv__section_hardware_wallet and settings__adv__trezor are used exclusively inside an if (isDevModeEnabled) block in AdvancedSettingsScreen.kt. Per the rule, these should be hardcoded inline rather than extracted as localized resources. Additionally, settings__adv__trezor (value: "Trezor") is a product name/acronym, which the same rule explicitly prohibits localizing.

CLAUDE.md: repository-layer suspend functions missing withContext(ioDispatcher)

Per CLAUDE.md: "ALWAYS wrap suspend functions in withContext(ioDispatcher) if in domain layer, using ctor injected prop @IoDispatcher private val ioDispatcher: CoroutineDispatcher".

TrezorRepo is the repository (domain) layer but:

1. The constructor does not inject @IoDispatcher CoroutineDispatcher — compare to WalletRepo and LightningRepo which both inject @BgDispatcher.
2. None of the suspend functions (initialize, scan, connect, getAddress, signTx, etc.) wrap their bodies in withContext(ioDispatcher).

This is particularly relevant because TrezorRepo performs direct I/O (SharedPreferences reads/writes in loadKnownDevices/saveKnownDevices, file I/O in logCredentialFileState) on whatever thread the caller happens to be on.